Skip to content

feat(updater): allow to skip secrets sync for branching - #2932

Closed
avallete wants to merge 4 commits into
developfrom
fix/skip-auth-secrets-syncing
Closed

feat(updater): allow to skip secrets sync for branching#2932
avallete wants to merge 4 commits into
developfrom
fix/skip-auth-secrets-syncing

Conversation

@avallete

@avallete avallete commented Nov 29, 2024

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

On branching side we'll be able to do:

updater.skipSecrets = true

What is the current behavior?

Please link any relevant issues here.

What is the new behavior?

Feel free to include screenshots if it includes visual changes.

Additional context

Add any other context or screenshots.

@coveralls

coveralls commented Nov 29, 2024

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 12114823667

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 10 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.06%) to 59.555%

Files with Coverage Reduction New Missed Lines %
internal/storage/rm/rm.go 2 89.53%
internal/debug/postgres.go 3 64.86%
internal/gen/keys/keys.go 5 12.9%
Totals Coverage Status
Change from base Build 12114343815: -0.06%
Covered Lines: 6398
Relevant Lines: 10743

💛 - Coveralls

@avallete
avallete force-pushed the fix/skip-auth-secrets-syncing branch from c3a9f62 to 6a90beb Compare November 29, 2024 15:54
@avallete avallete changed the title wip: add secrets exclusion feat(updater): allow to skip secrets sync for branching Nov 29, 2024
@avallete
avallete marked this pull request as ready for review November 29, 2024 15:56
Comment thread pkg/config/updater.go Outdated
@avallete
avallete force-pushed the fix/skip-auth-secrets-syncing branch from 676f3b4 to 189f294 Compare November 29, 2024 17:44
Comment thread pkg/config/auth.go
)

func (a *auth) StripUpdateBodySecrets(body v1API.UpdateAuthConfigBody) v1API.UpdateAuthConfigBody {
body.SmtpPass = nil

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately most of these secrets are required fields that we validate on api side when something like smtp is enabled. Setting them to nil would essentially block everyone from updating the whole config block.

An alternative is to let users opt out of those configs by turning the struct into pointers. Similar to #2954

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed I tested with a deployed project, and in such case the update went through but at the "first deploy" if no password is setup already then it'll fail.

@avallete avallete closed this Dec 9, 2024
@sweatybridge
sweatybridge deleted the fix/skip-auth-secrets-syncing branch December 30, 2024 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants